docs: sync README and ENV with Neon migration, fix tool count and stale error hints#74
Conversation
…le error hints - Replace Supabase with Neon as the database backend throughout docs: Quick Start §2 now points to neon.tech and uses psql to run schema scripts - Update config table: SUPABASE_URL/SUPABASE_SERVICE_KEY → DATABASE_URL/DATABASE_URL_UNPOOLED - Add missing env vars: OAuth (5 vars), CHUNKING_MODE, SEMANTIC_SIMILARITY_THRESHOLD, REDIS_URL - Fix tool count: 25 → 26 (extract_memories was undercounted) - Add setup-db-conversation-google.sql to Conversation Tools section - Replace Supabase badge with Neon badge - Fix 20 stale configError hints in tool files: 'Set SUPABASE_URL and SUPABASE_SERVICE_KEY' → 'Set DATABASE_URL' - Update docs/ENV.md core variables table to match config.ts https://claude.ai/code/session_011FNY15Sayo7kAekYo1wVcK
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughDocumentation and environment guidance were converted from Supabase-specific instructions to a PostgreSQL/Neon-centered setup: README and docs/ENV.md now instruct using 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/insights.ts (1)
80-83:⚠️ Potential issue | 🟠 MajorUse structured
toolError(toolName, error, context?)form instead of legacy single-arg pattern.Lines 80-83, 205-208, and 287-290 use the legacy
toolError(message)form. Per coding guidelines, all tool errors insrc/tools/*.tsmust use the structured formtoolError(toolName, error, context?)to provide proper context classification.Replace:
toolError(\Insight schema not initialized: ${schema.error}`)`with:
toolError('get_insights', new Error(schema.error))toolError('discover_connections', new Error(schema.error))toolError('dismiss_insight', new Error(schema.error))The structured form enables proper error logging and prevents LLM retry spirals by including the tool name and error context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/insights.ts` around lines 80 - 83, Replace the legacy single-argument toolError calls that interpolate schema.error with the structured form toolError(toolName, error, context?). Specifically, where you currently call toolError(`Insight schema not initialized: ${schema.error}`) in the three places handling schema checks, change them to use the tool names 'get_insights', 'discover_connections', and 'dismiss_insight' respectively and pass a new Error(schema.error) as the second argument (e.g., toolError('get_insights', new Error(schema.error))); keep or add an optional context object if useful. Ensure you update the three occurrences that check the schema (the ensureSchema() result handling in the get_insights, discover_connections, and dismiss_insight flows) to this structured pattern.
🧹 Nitpick comments (1)
docs/ENV.md (1)
15-16: Minor doc duplication:DATABASE_URL_UNPOOLEDappears twice.
DATABASE_URL_UNPOOLEDis listed in Core variables (Line 16) and again in Optional / advanced (Line 51). This can confuse users about which section is authoritative.Suggested cleanup (pick one source of truth)
## Optional / advanced @@ -| `DATABASE_URL_UNPOOLED` | Direct Postgres connection for pg_analyze tools and migrations | | `REDIS_URL` | Shared rate limiting across instances |(If you want to keep the extra detail under Optional/advanced, then consider removing/shortening the Core row instead.)
Also applies to: 51-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ENV.md` around lines 15 - 16, Remove the duplicated DATABASE_URL_UNPOOLED entry by choosing one authoritative location: either keep the full entry in "Core variables" and delete the row under "Optional / advanced", or retain the detailed explanation under "Optional / advanced" and shorten/remove the "Core variables" row; update the remaining entry for DATABASE_URL_UNPOOLED to include the intended note about unpooled direct connections for migrations so there is a single clear source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 86-99: The README still mixes Neon/Postgres setup with leftover
Supabase references; remove or replace all Supabase-specific text (e.g.,
headings and phrases like "Supabase Requirements", "independent of Supabase
client", “instead of using Supabase”, and any Supabase-specific troubleshooting
rows) so the doc consistently instructs Neon/Postgres usage under the existing
"Set Up Your Database" section and its script references (scripts/setup-db.sql,
setup-db-ollama.sql, setup-db-ollama-v2.sql, setup-db-google.sql,
setup-db-memory.sql, setup-db-conversation.sql, scripts/security-rls.sql); also
consolidate duplicate or conflicting sections (notably the areas around the
existing Neon instructions and the later Supabase blocks) into a single coherent
setup and troubleshooting section that only mentions Neon/Postgres and updates
any example commands/environment guidance to use DATABASE_URL and Neon
terminology.
In `@src/tools/memory.ts`:
- Around line 803-805: The inline comment above the branch that checks
storeResults && !isDatabaseConfigured() is outdated (mentions Supabase) — update
it to reference generic Postgres/database configuration; locate the comment near
the isDatabaseConfigured() check in src/tools/memory.ts (the block returning
configError('Database', 'Set DATABASE_URL')) and replace the "Only require
Supabase..." text with something like "Only require Postgres/database when
storing results" or another concise phrase indicating a generic database
requirement.
---
Outside diff comments:
In `@src/tools/insights.ts`:
- Around line 80-83: Replace the legacy single-argument toolError calls that
interpolate schema.error with the structured form toolError(toolName, error,
context?). Specifically, where you currently call toolError(`Insight schema not
initialized: ${schema.error}`) in the three places handling schema checks,
change them to use the tool names 'get_insights', 'discover_connections', and
'dismiss_insight' respectively and pass a new Error(schema.error) as the second
argument (e.g., toolError('get_insights', new Error(schema.error))); keep or add
an optional context object if useful. Ensure you update the three occurrences
that check the schema (the ensureSchema() result handling in the get_insights,
discover_connections, and dismiss_insight flows) to this structured pattern.
---
Nitpick comments:
In `@docs/ENV.md`:
- Around line 15-16: Remove the duplicated DATABASE_URL_UNPOOLED entry by
choosing one authoritative location: either keep the full entry in "Core
variables" and delete the row under "Optional / advanced", or retain the
detailed explanation under "Optional / advanced" and shorten/remove the "Core
variables" row; update the remaining entry for DATABASE_URL_UNPOOLED to include
the intended note about unpooled direct connections for migrations so there is a
single clear source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3d0aa46-c63f-4b28-8cf1-84049dfb14a8
📒 Files selected for processing (11)
README.mddocs/ENV.mdsrc/tools/ask.tssrc/tools/briefing.tssrc/tools/conversation.tssrc/tools/document.tssrc/tools/insights.tssrc/tools/memory.tssrc/tools/note.tssrc/tools/timeline.tssrc/tools/url.ts
… insights
README.md:
- Rename "Supabase Requirements" doc link → "Database Sizing"
- Remove "independent of Supabase client" from Postgres Analysis description
- Remove "instead of using Supabase" from Local Database section
- Replace two Supabase-specific troubleshooting rows with a generic DATABASE_URL hint
src/tools/memory.ts:
- Update inline comment: "Only require Supabase..." → "Only require Postgres when storing results"
src/tools/insights.ts:
- Replace three single-argument toolError() schema-error calls with the structured
two-argument form: toolError('get_insights'/'discover_connections'/'dismiss_insight', new Error(schema.error))
docs/ENV.md:
- Remove duplicate DATABASE_URL_UNPOOLED row from Core variables (canonical entry remains in Optional/advanced)
https://claude.ai/code/session_011FNY15Sayo7kAekYo1wVcK
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tools/insights.ts`:
- Around line 80-83: The code currently treats ensureSchema() failures as
runtime tool errors by calling toolError('get_insights', ...); instead, surface
these as configuration errors using configError(...) because schema/database
failures are permanent and should not trigger retries; replace calls to
toolError(...) that pass ensureSchema().error (e.g., in the get_insights flow
and the other occurrences around the ensureSchema checks) with
configError('get_insights', new Error(schema.error)) so the caller knows this is
a configuration/setup issue rather than a retryable runtime fault.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a6cb31c-1052-43ab-a8de-b96246221dff
📒 Files selected for processing (4)
README.mddocs/ENV.mdsrc/tools/insights.tssrc/tools/memory.ts
✅ Files skipped from review due to trivial changes (1)
- src/tools/memory.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/ENV.md
- README.md
…rror
Schema/database failures are permanent setup issues — callers should not
retry them. Replace the three toolError(...) calls on ensureSchema().error
with configError('Insight schema', schema.error) so the response carries
code:'CONFIG_ERROR' and the "do not retry" message.
https://claude.ai/code/session_011FNY15Sayo7kAekYo1wVcK
Quick Start §2 now points to neon.tech and uses psql to run schema scripts
https://claude.ai/code/session_011FNY15Sayo7kAekYo1wVcK